Skip to content

Issue #1721: Improve exceptional behavior in reactive streams #1723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2020

Conversation

n-miles
Copy link
Contributor

@n-miles n-miles commented May 8, 2020

Fixes #1721

Changes:

  • Errors on the request are now propagated to reactive subscribers instead of just to the request's ListenableFuture
  • Read timeouts can no longer occur if a reactive streams subscriber has no outstanding request. Note that this does not affect request timeouts - only read timeouts.
  • Cleaned up some random styling things

The original purpose of this change was to fix the read timeouts, but in the process of testing my change, I discovered that errors are not currently propagated to the subscriber, so I changed that as well.

I can split the changes into separate commits if you'd like.

* Errors on the request are now propagated to reactive subscribers
  instead of just to the request's ListenableFuture
* Read timeouts can no longer occur if a reactive streams subscriber has
  no outstanding request. Note that this does not affect request
  timeouts - only read timeouts.
@n-miles n-miles force-pushed the issue-1721-falsetimeouts branch from eae757b to cef8a26 Compare May 8, 2020 18:46
@n-miles
Copy link
Contributor Author

n-miles commented May 13, 2020

@slandelle have you had a chance to look at this change?

@slandelle
Copy link
Contributor

Honestly, I'm not familiar with reactive streams. Code style and test LGTM.

@slandelle slandelle merged commit 9c8c70d into AsyncHttpClient:master May 14, 2020
@n-miles n-miles deleted the issue-1721-falsetimeouts branch May 14, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False timeouts when using StreamedAsyncHandler
2 participants